Skip to content

Conversation

@brianquinlan
Copy link
Contributor

  • Thanks for your contribution! Please replace this text with a description of what this PR is changing or adding and why, list any relevant issues, and review the contribution guidelines below.

  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Contribution guidelines:

Many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

Note: The Dart team is trialing Gemini Code Assist. Don't take its comments as final Dart team feedback. Use the suggestions if they're helpful; otherwise, wait for a human reviewer.

@brianquinlan brianquinlan marked this pull request as draft October 27, 2025 22:52
@brianquinlan brianquinlan marked this pull request as ready for review November 4, 2025 03:42
@brianquinlan brianquinlan requested a review from aam November 4, 2025 03:51
(attr == NULL) ? NULL : (pthread_condattr_t *)attr->_pthread_condattr_t);
*err = errno;
if (r == 0) {
cond->_pthread_cond_t = c;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a need to assert that cond has not been already initialized? Calling this function more than once against same cond will result in a memory leak, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, POSIX allows me to do that ;-)

I think that I capture the behavior required by the standard:

Attempting to initialise an already initialised condition variable results in undefined behaviour.

The function pthread_cond_destroy() destroys the given condition variable specified by cond; the object becomes, in effect, uninitialised. An implementation may cause pthread_cond_destroy() to set the object referenced by cond to an invalid value. A destroyed condition variable object can be re-initialised using pthread_cond_init(); the results of otherwise referencing the object after it has been destroyed are undefined.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just meant that here(and in other methods below where we do calloc) the behavior will be well-defined in that aspect that it will create memory leaks.
But yeah, perhaps since it's posix "undefined behavior" territory, it might be not too terrible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would I tell if this is the first initialization or not? If the user uses malloc then the contents of the struct could be anything.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure. I think I just wished that there would be no malloc/free in these methods. But I don't have a proposal on how to avoid them neither.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to add this file too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a delete.

@brianquinlan brianquinlan merged commit 6ac7673 into dart-lang:main Nov 6, 2025
24 checks passed
@brianquinlan brianquinlan deleted the pthread branch November 6, 2025 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants